Skip to content

Implement snapshots#67

Open
TanmayArya-1p wants to merge 7 commits intosdslabs:refactorfrom
TanmayArya-1p:snapshot
Open

Implement snapshots#67
TanmayArya-1p wants to merge 7 commits intosdslabs:refactorfrom
TanmayArya-1p:snapshot

Conversation

@TanmayArya-1p
Copy link
Copy Markdown
Member

Addresses #38

}

#[test]
fn test_create_and_load_snapshot() {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these tests give an overview of how the snapshots api is to be used

Copy link
Copy Markdown
Collaborator

@4adex 4adex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix error handling in these certain places

.lock()
.unwrap()
.add_snapshot(&snapshot_path)
.unwrap();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Propagate error or panic if needed but don't use excessive unwraps. Also check other unwraps in engine. (unwraps are okay in tests only)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the unwraps in the engine except for the ones in the worker thread. not sure how to handle errors in the worker thread(do i panic?). I'll look into best practices for this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refactored error handling in worker thread in 76e3afb

"Storage checkpoint was not properly made".to_string(),
))?
.to_str()
.unwrap()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ok or instead of unwrap

@4adex
Copy link
Copy Markdown
Collaborator

4adex commented Mar 24, 2026

@TanmayArya-1p rebase and resolve conflicts

@TanmayArya-1p
Copy link
Copy Markdown
Member Author

@4adex fixed the merge conflicts. the snapshots crate and most of the snapshots related code does not use snafu for error handling, we need to address this later

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants